Skip to content

run the metacpan-api app.psgi, not the api.pl #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 1, 2024
Merged

Conversation

haarg
Copy link
Member

@haarg haarg commented Sep 30, 2024

Currently, metacpan-api has an app.psgi for the Catalyst app. This has most of the end points. There is also an api.pl script which runs Mojolicious. This hosts a Minion admin interface, a few end points which are duplicates of the Catalyst end points, and mounts the app.psgi to handle everything else.

This setup uses the Mojolicious MountPSGI plugin. This plugin is rather limited and has some issues. It ends up loading the PSGI app the first time it is requested. This means in a prefork setup, none of the memory is shared. This is wasteful, but also means creating worker processes is very slow. And if it is slow enough, the worker won't respond to the heartbeat check, and the parent process will kill it before it is even ready.

The problems with MountPSGI could be fixed, but the Mojolicious app is also just adding complexity without providing any real value in its current state. The end points it provides are duplicates.

We aren't using the Minion integration to dispatch tasks anywhere in the API. And its admin interface doesn't really belong in the same service as the API server.

Simplify the way metacpan-api is hosted by just running the Catalyst app. This avoids the problems with the MountPSGI plugin. Replacing the Minion admin interface is a task left for a different time.

Using Mojolicous in the future would be reasonable, but it should be done as a proper conversion.

Currently, metacpan-api has an app.psgi for the Catalyst app. This has
most of the end points. There is also an api.pl script which runs
Mojolicious. This hosts a Minion admin interface, a few end points which
are duplicates of the Catalyst end points, and mounts the app.psgi to
handle everything else.

This setup uses the Mojolicious MountPSGI plugin. This plugin is rather
limited and has some issues. It ends up loading the PSGI app the first
time it is requested. This means in a prefork setup, none of the memory
is shared. This is wasteful, but also means creating worker processes is
very slow. And if it is slow enough, the worker won't respond to the
heartbeat check, and the parent process will kill it before it is even
ready.

The problems with MountPSGI could be fixed, but the Mojolicious app is
also just adding complexity without providing any real value in its
current state. The end points it provides are duplicates.

We aren't using the Minion integration to dispatch tasks anywhere in the
API. And its admin interface doesn't really belong in the same service
as the API server.

Simplify the way metacpan-api is hosted by just running the Catalyst
app. This avoids the problems with the MountPSGI plugin. Replacing the
Minion admin interface is a task left for a different time.

Using Mojolicous in the future would be reasonable, but it should be
done as a proper conversion.
@haarg haarg force-pushed the haarg/api-via-psgi branch from f3c400e to 159c6dd Compare September 30, 2024 21:18
@haarg haarg changed the title run the metacpan-api app.psgi, not the app.pl run the metacpan-api app.psgi, not the api.pl Sep 30, 2024
Copy link
Member

@oalders oalders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have the Minion UI at some point, but I think it hasn't been available for a long time. There have been a number of times where I've wanted to check the status of the queue.

@haarg haarg merged commit c00efa2 into master Oct 1, 2024
@haarg haarg deleted the haarg/api-via-psgi branch October 1, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants